fix: Make trait functions generic over Self
#3702
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Problem & Summary
Checking a trait function's type previously was very ad-hoc. When a
TraitMethodReference
was found, it'd manually create a function type from the trait function's definition and instantiate it - even though instantiating aType::Function
does nothing.I've changed the TraitFunction struct to add a
Type
field for the whole function type, which includes generics. The type now should usually be aType::Forall
. In addition, I've added the trait'sSelf
type to the list of generics in the Type::Forall.This fixes the issue where the
Self
type would be bound after certain function calls which would affect callsites but not a function body leading to a type mismatch between function return type and a call's return type. This is the issue we were encountering after adding traits to noir-protocol-circuits.Additional Information
No tests are provided since I was unable to make a minimal repro from the thousands of lines of noir-protocol-circuits unfortunately. I'll keep experimenting while this PR is up.
I'm also merging into jf/fix-3089 since this was somewhat accidentally built on top of it. The two changes are otherwise unrelated.
Documentation*
Check one:
PR Checklist*
cargo fmt
on default settings.